-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ChatGPTGenerator
#5692
feat: ChatGPTGenerator
#5692
Conversation
…to generators-module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the first pass I tried to find obvious and easy-to-find errors. Looks solid; left some early feedback.
headers = {"Authorization": f"Bearer {api_key}", "Content-Type": "application/json"} | ||
if openai_organization: | ||
headers["OpenAI-Organization"] = openai_organization | ||
url = f"{api_base_url}/chat/completions" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vblagoje We're using the chat completion endpoint for ChatGPT: https://platform.openai.com/docs/api-reference/chat/create
Pull Request Test Coverage Report for Build 6039895083
💛 - Coveralls |
:param top_p: An alternative to sampling with temperature, called nucleus sampling, where the model | ||
considers the results of the tokens with top_p probability mass. So 0.1 means only the tokens | ||
comprising the top 10% probability mass are considered. | ||
:param n: How many completions to generate for each prompt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we elaborate on what this means?
considers the results of the tokens with top_p probability mass. So 0.1 means only the tokens | ||
comprising the top 10% probability mass are considered. | ||
:param n: How many completions to generate for each prompt. | ||
:param stop: One or more sequences where the API will stop generating further tokens. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this define some specific phrase that tells the API to stop generating tokens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the contrary: it's a keyword that tells us that we can discard all the LLM output that comes after it. It used to be quite important as ChatGPT would keep generating after the stopword if not instructed correctly. I'm not sure it's still an issue, but the parameter is still present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the wording to try make it more clear, it was quite ambiguous indeed
:param api_base_url: The OpenAI API Base url, defaults to `https://api.openai.com/v1`. | ||
:param openai_organization: The OpenAI organization ID. | ||
|
||
See OpenAI documentation](https://platform.openai.com/docs/api-reference/chat) for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See OpenAI documentation](https://platform.openai.com/docs/api-reference/chat) for more details. | |
See OpenAI [documentation](https://platform.openai.com/docs/api-reference/chat) for more details. |
This PR has become really big, so I'll split it into two smaller ones. I'll make sure to mark you two as reviewers of those PRs as well. |
Related Issues
generators
(2.0) #5690Proposed Changes:
ChatGPTGenerator
according to the LLM ProposalHow did you test it?
Notes for the reviewer
n/a
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.